Skip to content

Provide a specialized interval tree for resource management#3

Closed
jiangliu wants to merge 7 commits intorust-vmm:mainfrom
jiangliu:v1
Closed

Provide a specialized interval tree for resource management#3
jiangliu wants to merge 7 commits intorust-vmm:mainfrom
jiangliu:v1

Conversation

@jiangliu
Copy link
Member

@jiangliu jiangliu commented Oct 28, 2019

This PR implements a specialized version of interval tree for resource management, which could be used manage normal VMM resources.

@jiangliu
Implement a simple interval tree
2c5d6b6 
Implement a simple interval tree with generic methods insert()/delete()
to prepare for resource management.
@jiangliu jiangliu mentioned this pull request Oct 29, 2019
Merged
liujing2
liujing2 reviewed Oct 29, 2019
View reviewed changes
liujing2
liujing2 reviewed Oct 29, 2019
View reviewed changes
src/lib.rs Outdated

impl Constraint {
/// Create a new constraint object with default settings.
pub fn new(size: u64) -> Self {
Copy link

@liujing2 liujing2 Oct 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this function needs more parameters?

Copy link
Member Author

@jiangliu jiangliu Oct 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used to cover the most common usage scenario. For rare use cases, you may dirrectly construct a Constraint object or we may add some new interfaces for those cases.

Copy link

@liujing2 liujing2 Oct 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what is the difference between this and ResourceConstraint defined in rust-vmm/vm-device#9 ?

Copy link
Member Author

@jiangliu jiangliu Oct 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's two related abstractions. With Device::ResourceConstraint and vm_allocator::Constraint, there's still one piece missing of the whole puzzle. We need the vmm to implement a resource manager, which converts Device::ResourceConstraint to vm_allocator::Constraint.

  1. vm_allocator is just a generic resource allocator,
  2. vmm::ResourceManager needs to define type of resources and use vm_allocator to manage its resources.
    It's still unclear whether vmm::ResourceManager should be VMM specific or we need another vmm_resource crate.

Copy link

@liujing2 liujing2 Oct 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's two related abstractions. With Device::ResourceConstraint and vm_allocator::Constraint, there's still one piece missing of the whole puzzle. We need the vmm to implement a resource manager, which converts Device::ResourceConstraint to vm_allocator::Constraint.

Ok, this is the "glue" part between vm-device and vm-allocator.

  1. vm_allocator is just a generic resource allocator,
  2. vmm::ResourceManager needs to define type of resources and use vm_allocator to manage its resources.
    It's still unclear whether vmm::ResourceManager should be VMM specific or we need another vmm_resource crate.

I think we also need a conversion from the vmm::ResourceManager::allocate_resources() return type to vm_device::Resource for registering devices.

Copy link
Member Author

@jiangliu jiangliu Oct 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true. Interval tree just manages a set of IDs, the resource manager needs to define the specific resources the ID stands for.

@sameo
Copy link

sameo commented Nov 7, 2019

Before looking deeper into the actual code, I guess my first general question would be: why couldn't we use one of the existing rust collection structure as the inner IntervalTree tree structure:

struct IntervalTree<T> {
          root: BTreeMap<Range, T>>,          
}

@jiangliu
Copy link
Member Author

jiangliu commented Nov 7, 2019 •
edited
Loading

Before looking deeper into the actual code, I guess my first general question would be: why couldn't we use one of the existing rust collection structure as the inner IntervalTree tree structure:

struct IntervalTree<T> {
          root: BTreeMap<Range, T>>,          
}

IntervalTree supports using range(start, end) as the key, so we could search the entry containing a specific value(point).
We want to avoid this

    fn first_before(&self, addr: u64) -> Option<(BusRange, &Mutex<dyn BusDevice>)> {
        // for when we switch to rustc 1.17: self.devices.range(..addr).iter().rev().next()
        for (range, dev) in self.devices.iter().rev() {
            if range.0 <= addr {
                return Some((*range, dev));
            }
        }
        None
    }

@jiangliu
Add more interval tree helpers
c965f7e 
Add more interval tree helper methods to support resource management.

Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@jiangliu
Describe resource allocation constraints
5d8b020 
Introduce struct Constraint to describe resource allocation constraints.

Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@jiangliu
Implement IntervalTree::allocate()/free()
b2a9e8d 
Implement IntervalTree::allocate()/free() to allocate/free resources.

Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@jiangliu
Add more unit tests
199beeb 
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@jiangliu
Add usage example for the interval tree.
8a32f2c 
Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@sameo sameo mentioned this pull request Dec 20, 2019
Merged
@jiangliu
Add method to get mutable reference of nodes
f05dcd7 
Add several methods to get mutable reference to data in the interval
tree.

Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>
@AlexandruCihodaru
Copy link
Collaborator

AlexandruCihodaru commented Apr 7, 2022

Closing this as it is obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreeaflorescu andreeaflorescu Awaiting requested review from andreeaflorescu

@bonzini bonzini Awaiting requested review from bonzini

@sameo sameo Awaiting requested review from sameo

1 more reviewer

@liujing2 liujing2 liujing2 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jiangliu @sameo @AlexandruCihodaru @liujing2